Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds a patchfile for fsevents #692

Merged
merged 17 commits into from
Jan 15, 2020
Merged

Adds a patchfile for fsevents #692

merged 17 commits into from
Jan 15, 2020

Conversation

arcanis
Copy link
Member

@arcanis arcanis commented Jan 14, 2020

What's the problem this PR addresses?

FsEvents isn't aware of the virtual filesystem ($$virtual, more details at the end of this post), and that breaks Webpack when requiring files from a workspace that lists peer dependencies.

How did you fix it?

As a short-term fix I've implemented the fix in fsevents via our new patch: protocol. I wonder if we could convince the fsevents maintainers to merge the change upstream? (cc @paulmillr / @pipobscure)

What is this $$virtual thing?

First read that. Now, as the article says:

Virtual packages [...] are now implemented through a virtual filesystem layer. This circumvents the need to create hundreds of confusing symlinks, improving compatibility with Windows and preventing issues that would arise with third-party tools calling realpath

The way we do this by injecting a fs layer that applies the following algorithm:

  • Find occurances of /$$virtual/<whatever>/<number>/
  • Replace them by '../'.repeat(number)

So for example, /foo/$$virtual/abcdef/0/hello becomes /foo/hello, and /foo/$$virtual/abcdef/1/hello becomes /hello. This technic allows us to generate an infinity of paths for a given package.

@paulmillr
Copy link

Could you get the PR to our fsevents repo? We will discuss it there!

@pipobscure
Copy link

Though I have to say that fsevents seems like the worst possible layer to do this.

@arcanis
Copy link
Member Author

arcanis commented Jan 15, 2020

Damn, for some reasons fsevents just doesn't seem to work on our GitHub Actions, even though you actually use it in your own tests 🙁

@arcanis
Copy link
Member Author

arcanis commented Jan 15, 2020

Oh interesting - FSEvents 2.x works on GitHub Actions, but not 1.x 🤔

@arcanis
Copy link
Member Author

arcanis commented Jan 15, 2020

Errata: I had a typo in the variable name that was causing undefined to be sent to FsEvents, triggering a segfault. For some reason it still worked on my computer (the segfault's ways are mysterious indeed), but that's why it failed once on CI. Now everything's passing!

@arcanis arcanis merged commit d955efa into master Jan 15, 2020
@arcanis arcanis deleted the mael/fsevents branch January 15, 2020 11:39
@arcanis
Copy link
Member Author

arcanis commented Jan 15, 2020

Merging for now, will open a PR against FSEvents later.

bpierre added a commit to aragon/court-dashboard that referenced this pull request Feb 6, 2020
Waiting for copy-aragon-ui-assets to be compatible with PnP.

Also waiting for the next yarn version to be released to get this fix: yarnpkg/berry#692

Alternatively, we could upgrade react-scripts.
bpierre added a commit to aragon/court-dashboard that referenced this pull request Feb 6, 2020
Waiting for copy-aragon-ui-assets to be compatible with PnP.

Also waiting for the next yarn version to be released to get this fix: yarnpkg/berry#692

Alternatively, we could upgrade react-scripts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants